-
Notifications
You must be signed in to change notification settings - Fork 112
Diving Improvements #697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Diving Improvements #697
Conversation
… on the objective pseudcost estimate.
… on the objective pseudcost estimate.
📝 WalkthroughWalkthroughAdds diving heuristics and a thread-safe node_queue, removes legacy diving_queue, extends basis/refactor APIs to accept lower/upper bounds, introduces diving configuration and pseudo-cost/objective estimates, and rewrites B&B threading/queueing and many related call sites across dual_simplex and MIP modules. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cpp/src/dual_simplex/phase2.cpp (1)
2837-2858: Passprimal_infeasibility_squaredinstead ofprimal_infeasibilitytoupdate_single_primal_infeasibilityat line 2858.The previous two calls to
update_primal_infeasibilities(lines 2837 and 2849) correctly accumulate squared infeasibility changes intoprimal_infeasibility_squared. However,update_single_primal_infeasibilityalso operates on squared infeasibilities internally (computinginfeas * infeasand accumulating viaprimal_inf + (new_val - old_val)), so it should update the same accumulator. Passingprimal_infeasibilityinstead breaks infeasibility tracking—the entering variable's squared infeasibility delta is accumulated into the wrong variable, leavingprimal_infeasibility_squaredincomplete.
🧹 Nitpick comments (8)
cpp/src/mip/diversity/recombiners/sub_mip.cuh (1)
108-114: Diving heuristics temporarily restricted with clear intent.The explicit configuration disables most diving heuristics (guided, coefficient, pseudocost) for SubMIP, restricting to line search diving only. The comment clearly indicates this is a temporary restriction pending future enablement of all heuristics.
Consider extracting this diving configuration pattern into a helper function since the same configuration appears in multiple files (sub_mip.cuh and rins.cu). This would reduce duplication and make future updates easier.
// Example helper static auto create_line_search_only_diving_settings() { diving_heuristics_settings_t settings; settings.num_diving_tasks = 1; settings.disable_guided_diving = true; settings.disable_coefficient_diving = true; settings.disable_pseudocost_diving = true; return settings; }Based on coding guidelines: refactor code duplication in solver components.
cpp/src/mip/diversity/lns/rins.cu (1)
258-270: Branch-and-bound settings correctly updated for new diving API.The configuration properly integrates the new
diving_settingsstructure while preserving existing behavior for threading, tolerances, logging, and callbacks. The temporary restriction to line search diving only is clearly documented.This file contains the same diving configuration pattern as
sub_mip.cuh. Consider consolidating this repeated configuration into a shared helper function to reduce duplication.Based on coding guidelines: refactor code duplication in solver components (3+ occurrences).
cpp/src/dual_simplex/diving_heuristics.hpp (1)
10-13: Consider adding explicit logger include.
logger_t&is used in all function signatures, but the header relies on transitive inclusion throughpseudo_costs.hpp. For better header self-containment and faster compile times, consider adding an explicit include or forward declaration:#include <dual_simplex/basis_updates.hpp> #include <dual_simplex/bounds_strengthening.hpp> #include <dual_simplex/pseudo_costs.hpp> +#include <dual_simplex/logger.hpp> #include <vector>cpp/src/dual_simplex/basis_solves.cpp (1)
663-671: Logic flow issue: condition at line 667 may be unreachable in practice.The current logic:
- Line 663: If
lower == -inf && upper == inf→ FREE- Line 665: Else if
lower > -inf→ LOWER- Line 667: Else if
upper < inf→ UPPER- Line 669: Else → assert
At line 667, we know
lower[bad_j] <= -inf(from failing line 665). Combined with failing line 663, we must haveupper[bad_j] < inf, so the condition is always true when reached. Theelseat line 669 is dead code.Consider simplifying:
if (lower[bad_j] == -inf && upper[bad_j] == inf) { vstatus[bad_j] = variable_status_t::NONBASIC_FREE; } else if (lower[bad_j] > -inf) { vstatus[bad_j] = variable_status_t::NONBASIC_LOWER; - } else if (upper[bad_j] < inf) { - vstatus[bad_j] = variable_status_t::NONBASIC_UPPER; } else { - assert(1 == 0); + vstatus[bad_j] = variable_status_t::NONBASIC_UPPER; }The assert is unnecessary since the logic is exhaustive.
cpp/src/dual_simplex/diving_heuristics.cpp (1)
49-55: Edge case: emptyfractionalvector causes undefined behavior.If
fractionalis empty, the fallback at line 50 accessesfractional[0]which is undefined, and the assertions at lines 54-55 will fail. While callers should ensurefractionalis non-empty, defensive handling would improve robustness:if (round_dir == rounding_direction_t::NONE) { + assert(!fractional.empty() && "line_search_diving called with empty fractional list"); branch_var = fractional[0]; round_dir = rounding_direction_t::DOWN; } - - assert(round_dir != rounding_direction_t::NONE); - assert(branch_var >= 0);cpp/src/dual_simplex/node_queue.hpp (3)
33-38: Incorrect use ofstd::forwardwith doubly-referenced type.The
Args&&in the template parameter already declares forwarding references. Usingstd::forward<Args&&>adds an extra&&, which is redundant and could cause issues with some compilers.template <typename... Args> void emplace(Args&&... args) { - buffer.emplace_back(std::forward<Args&&>(args)...); + buffer.emplace_back(std::forward<Args>(args)...); std::push_heap(buffer.begin(), buffer.end(), comp); }
136-146: Potential integer type narrowing in size accessors.
heap_t::size()returnssize_t, butdiving_queue_size()andbest_first_queue_size()returni_t(typicallyint). For very large heaps exceedingINT_MAX, this could cause truncation.Consider returning
size_tor usingstatic_castwith appropriate bounds checking:- i_t diving_queue_size() + size_t diving_queue_size() { std::lock_guard<omp_mutex_t> lock(mutex); return diving_heap.size(); } - i_t best_first_queue_size() + size_t best_first_queue_size() { std::lock_guard<omp_mutex_t> lock(mutex); return best_first_heap.size(); }
116-134: Stale entries indiving_heapcause O(n) iterations inpop_diving().Both
best_first_heapanddiving_heapreference the sameheap_entry_tobjects viashared_ptr. Whenpop_best_first()executesstd::exchange(entry.value()->node, nullptr)at line 110, it nullifies the entry's node pointer. However, the entry itself remains indiving_heap. Subsequent calls topop_diving()must iterate through these stale entries (lines 122–130) until finding a valid one, resulting in O(n) complexity where n is the number of stale entries accumulated.Consider maintaining a count of valid entries or implementing atomic removal from both heaps to avoid worst-case iteration when heavy best-first activity precedes diving operations.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
cpp/src/dual_simplex/CMakeLists.txt(1 hunks)cpp/src/dual_simplex/basis_solves.cpp(3 hunks)cpp/src/dual_simplex/basis_solves.hpp(1 hunks)cpp/src/dual_simplex/basis_updates.cpp(2 hunks)cpp/src/dual_simplex/basis_updates.hpp(1 hunks)cpp/src/dual_simplex/bounds_strengthening.cpp(2 hunks)cpp/src/dual_simplex/branch_and_bound.cpp(30 hunks)cpp/src/dual_simplex/branch_and_bound.hpp(8 hunks)cpp/src/dual_simplex/crossover.cpp(3 hunks)cpp/src/dual_simplex/diving_heuristics.cpp(1 hunks)cpp/src/dual_simplex/diving_heuristics.hpp(1 hunks)cpp/src/dual_simplex/diving_queue.hpp(0 hunks)cpp/src/dual_simplex/logger.hpp(1 hunks)cpp/src/dual_simplex/mip_node.hpp(4 hunks)cpp/src/dual_simplex/node_queue.hpp(1 hunks)cpp/src/dual_simplex/phase2.cpp(12 hunks)cpp/src/dual_simplex/primal.cpp(1 hunks)cpp/src/dual_simplex/pseudo_costs.cpp(4 hunks)cpp/src/dual_simplex/pseudo_costs.hpp(1 hunks)cpp/src/dual_simplex/simplex_solver_settings.hpp(3 hunks)cpp/src/mip/diversity/lns/rins.cu(1 hunks)cpp/src/mip/diversity/recombiners/sub_mip.cuh(1 hunks)cpp/src/mip/solver.cu(1 hunks)
💤 Files with no reviewable changes (1)
- cpp/src/dual_simplex/diving_queue.hpp
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{cu,cuh,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh,cpp,hpp,h}: Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Check that hard-coded GPU de...
Files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.hppcpp/src/dual_simplex/logger.hppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cucpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/mip_node.hppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
**/*.{h,hpp,py}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Verify C API does not break ABI stability (no struct layout changes, field reordering); maintain backward compatibility in Python and server APIs with deprecation warnings
Files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/basis_solves.hppcpp/src/dual_simplex/logger.hppcpp/src/dual_simplex/mip_node.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hpp
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cpp,hpp,h}: Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Validate input sanitization to prevent buffer overflows and resource exhaustion attacks; avoid unsafe deserialization of problem files
Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.hppcpp/src/dual_simplex/logger.hppcpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/mip_node.hppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cu,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.hppcpp/src/dual_simplex/logger.hppcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cucpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/mip_node.hppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cu,cuh}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh}: Every CUDA kernel launch and memory operation must have error checking with CUDA_CHECK or equivalent verification
Avoid reinventing functionality already available in Thrust, CCCL, or RMM libraries; prefer standard library utilities over custom implementations
Files:
cpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cu
**/*.cu
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.cu: Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Detect inefficient GPU kernel launches with low occupancy or poor memory access patterns; optimize for coalesced memory access and minimize warp divergence in hot paths
Files:
cpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cu
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Applied to files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.hppcpp/src/mip/solver.cucpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Applied to files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.hppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/dual_simplex/CMakeLists.txtcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cucpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/mip_node.hppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Applied to files:
cpp/src/dual_simplex/basis_updates.hppcpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.hppcpp/src/dual_simplex/CMakeLists.txtcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cucpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Applied to files:
cpp/src/dual_simplex/bounds_strengthening.cppcpp/src/mip/solver.cucpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Applied to files:
cpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/CMakeLists.txtcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cucpp/src/dual_simplex/primal.cppcpp/src/dual_simplex/crossover.cppcpp/src/dual_simplex/basis_updates.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Applied to files:
cpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/basis_solves.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Test with free variables, singleton problems, and extreme problem dimensions near resource limits to validate edge case handling
Applied to files:
cpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/CMakeLists.txt
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Applied to files:
cpp/src/dual_simplex/bounds_strengthening.cppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Applied to files:
cpp/src/dual_simplex/CMakeLists.txtcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cucpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh} : Avoid reinventing functionality already available in Thrust, CCCL, or RMM libraries; prefer standard library utilities over custom implementations
Applied to files:
cpp/src/dual_simplex/CMakeLists.txt
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check that hard-coded GPU device IDs and resource limits are made configurable; abstract multi-backend support for different CUDA versions
Applied to files:
cpp/src/dual_simplex/CMakeLists.txtcpp/src/mip/diversity/lns/rins.cucpp/src/mip/solver.cu
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Applied to files:
cpp/src/dual_simplex/CMakeLists.txt
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Applied to files:
cpp/src/dual_simplex/CMakeLists.txt
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*benchmark*.{cpp,cu,py} : Include performance benchmarks and regression detection for GPU operations; verify near real-time performance on million-variable problems
Applied to files:
cpp/src/dual_simplex/CMakeLists.txt
📚 Learning: 2025-12-04T20:09:09.264Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 602
File: cpp/src/linear_programming/solve.cu:732-742
Timestamp: 2025-12-04T20:09:09.264Z
Learning: In cpp/src/linear_programming/solve.cu, the barrier solver does not currently return INFEASIBLE or UNBOUNDED status. It only returns OPTIMAL, TIME_LIMIT, NUMERICAL_ISSUES, or CONCURRENT_LIMIT.
Applied to files:
cpp/src/mip/solver.cucpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/phase2.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cpp,hpp,h} : Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Applied to files:
cpp/src/mip/solver.cucpp/src/dual_simplex/pseudo_costs.cpp
📚 Learning: 2025-12-04T04:11:12.640Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/src/dual_simplex/scaling.cpp:68-76
Timestamp: 2025-12-04T04:11:12.640Z
Learning: In the cuOPT dual simplex solver, CSR/CSC matrices (including the quadratic objective matrix Q) are required to have valid dimensions and indices by construction. Runtime bounds checking in performance-critical paths like matrix scaling is avoided to prevent slowdowns. Validation is performed via debug-only check_matrix() calls wrapped in #ifdef CHECK_MATRIX.
Applied to files:
cpp/src/dual_simplex/basis_solves.cppcpp/src/dual_simplex/branch_and_bound.hpp
🧬 Code graph analysis (11)
cpp/src/dual_simplex/basis_updates.hpp (1)
cpp/src/dual_simplex/presolve.cpp (3)
lower(240-240)upper(85-85)upper(241-241)
cpp/src/dual_simplex/logger.hpp (1)
cpp/src/dual_simplex/simplex_solver_settings.hpp (2)
log_filename(99-99)log_filename(99-99)
cpp/src/dual_simplex/primal.cpp (2)
cpp/src/dual_simplex/basis_solves.cpp (3)
basis_repair(614-675)basis_repair(614-622)basis_repair(860-868)cpp/src/dual_simplex/basis_solves.hpp (1)
basis_repair(43-51)
cpp/src/dual_simplex/crossover.cpp (2)
cpp/src/dual_simplex/basis_solves.cpp (3)
basis_repair(614-675)basis_repair(614-622)basis_repair(860-868)cpp/src/dual_simplex/basis_solves.hpp (1)
basis_repair(43-51)
cpp/src/dual_simplex/basis_updates.cpp (3)
cpp/src/dual_simplex/basis_solves.cpp (3)
basis_repair(614-675)basis_repair(614-622)basis_repair(860-868)cpp/src/dual_simplex/basis_solves.hpp (1)
basis_repair(43-51)cpp/src/dual_simplex/basis_updates.hpp (1)
A(374-380)
cpp/src/dual_simplex/basis_solves.cpp (1)
cpp/src/dual_simplex/presolve.cpp (3)
lower(240-240)upper(85-85)upper(241-241)
cpp/src/dual_simplex/diving_heuristics.hpp (2)
cpp/src/dual_simplex/pseudo_costs.hpp (2)
fractional(46-49)fractional(51-52)cpp/src/dual_simplex/simplex_solver_settings.hpp (2)
log(98-98)log(100-100)
cpp/src/dual_simplex/pseudo_costs.cpp (1)
cpp/src/dual_simplex/pseudo_costs.hpp (2)
fractional(46-49)fractional(51-52)
cpp/src/dual_simplex/phase2.cpp (1)
cpp/src/dual_simplex/primal.cpp (2)
primal_infeasibility(202-240)primal_infeasibility(202-205)
cpp/src/dual_simplex/node_queue.hpp (1)
cpp/src/dual_simplex/mip_node.hpp (4)
lower(97-108)lower(97-99)lower(112-130)lower(112-114)
cpp/src/dual_simplex/branch_and_bound.hpp (1)
cpp/src/dual_simplex/node_queue.hpp (4)
node(21-25)node(21-21)node(27-31)node(27-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.11, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.13, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.10, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.13, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.11, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-sh-client / 13.0.2, 3.10, amd64, rockylinux8
- GitHub Check: checks / check-style
🔇 Additional comments (39)
cpp/src/dual_simplex/CMakeLists.txt (1)
34-34: LGTM!The addition of
diving_heuristics.cppto the build aligns with the PR objectives to introduce new diving heuristic strategies.cpp/src/dual_simplex/bounds_strengthening.cpp (2)
157-164: Logging level correctly adjusted to debug.The change from
printftosettings.log.debug()appropriately reduces stdout noise while preserving diagnostic information for debugging. This is proper logging hygiene for infeasibility detection during bounds strengthening iterations.
214-216: Logging level correctly adjusted to debug.Consistent with the previous logging change, this infeasibility message is appropriately moved to debug level logging.
cpp/src/dual_simplex/primal.cpp (1)
301-309: All basis_repair call sites have been consistently updated with bound parameters.The call in
primal.cppat lines 301-309 correctly passeslp.lowerandlp.upperas the 3rd and 4th parameters. Verification confirms this pattern is consistent across all five basis_repair call sites in the codebase:
primal.cpp:301- passeslp.lower, lp.uppercrossover.cpp:788, 1142, 1341- all passlp.lower, lp.upperbasis_updates.cpp:2071- passeslower, upper(local variables in templated context)This ensures bounds-aware basis repair during algorithm phase transitions (simplex, crossover phases).
cpp/src/mip/solver.cu (1)
176-180: Threading configuration correctly refactored for new diving_settings structure.The code properly updates the thread distribution logic to use the new
diving_settings.num_diving_tasksfield instead of the previousnum_diving_threads. The thread allocation strategy remains unchanged (BFS threads = num_threads/4, diving tasks = remaining threads).cpp/src/dual_simplex/pseudo_costs.hpp (1)
46-49: API change correctly expands pseudo-cost functionality.The function signature update from
variable_selectiontovariable_selection_and_obj_estimateproperly reflects the expanded responsibility: returning both the selected variable index and an objective estimate. The addition of thelower_boundparameter enables more sophisticated objective estimation for diving heuristics.All call sites have been correctly updated. The old
variable_selectionfunction is preserved as a backward-compatibility wrapper that properly unpacks the returned pair viastd::tie, stores the objective estimate in node state for search tree ordering, and returns the branch decision. New call sites directly usevariable_selection_and_obj_estimatewith proper structured binding to capture both returned elements.cpp/src/dual_simplex/logger.hpp (1)
33-44: LGTM! Backward-compatible API refinement.The change from
std::stringtoconst char*for the mode parameter avoids unnecessary string construction when passing literals. The default value"w"ensures backward compatibility with existing callers likeset_log_filenamethat don't specify a mode.cpp/src/dual_simplex/basis_solves.hpp (1)
42-51: LGTM! Bound-aware basis repair signature.The addition of
lowerandupperbound vectors enablesbasis_repairto correctly assign variable status (NONBASIC_FREE, NONBASIC_LOWER, or NONBASIC_UPPER) for variables being removed from the basis. This is essential for maintaining dual feasibility during repair operations. Based on coding guidelines, this correctly validates variable bounds during algorithm state transitions.cpp/src/dual_simplex/crossover.cpp (3)
788-796: LGTM! Correct bound propagation to basis repair.The
lp.lowerandlp.upperbounds are correctly passed tobasis_repair, enabling proper variable status assignment during the primal push refactoring phase.
1142-1150: LGTM! Consistent bound handling in crossover initialization.Bounds are correctly propagated during the initial basis repair in the crossover function, consistent with the other call sites.
1341-1349: LGTM! Bounds passed after phase 1 completion.The third call site correctly passes bounds after phase 1 completes, maintaining consistency across all
basis_repairinvocations in the crossover workflow.cpp/src/dual_simplex/basis_updates.cpp (2)
2046-2054: LGTM! Signature update matches declaration.The
refactor_basisimplementation correctly accepts the newlowerandupperbound parameters, consistent with the header declaration inbasis_updates.hpp.
2071-2072: LGTM! Bounds correctly forwarded to basis_repair.When initial factorization fails, the bounds are properly passed to
basis_repairto enable correct variable status assignment during the repair process. Based on coding guidelines, this ensures correct initialization of variable bounds when transitioning between algorithm phases.cpp/src/dual_simplex/basis_updates.hpp (1)
374-380: Verify all callers ofrefactor_basishave been updated to pass the new bound parameters.The
refactor_basisdeclaration correctly addslowerandupperbound vectors as const references. All callers in phase2.cpp (lines 2248, 2910, 2917) have been properly updated to passlp.lowerandlp.upper. This is a breaking API change requiring all callers to be updated.cpp/src/dual_simplex/simplex_solver_settings.hpp (1)
22-34: Well-structured diving configuration.The new
diving_heuristics_settings_tstruct cleanly encapsulates diving-related parameters with sensible defaults. The separation of heuristic enable/disable flags and resource limits (node_limit, iteration_limit_factor, backtrack) follows good configuration design practices.cpp/src/dual_simplex/pseudo_costs.cpp (2)
202-202: Good improvement: RAII-based locking.Using
std::lock_guard<omp_mutex_t>ensures the mutex is properly released even if an exception occurs or the function returns early. This is a safer pattern than manual lock/unlock.
256-321: Correct implementation of pseudocost-based objective estimate.The function correctly:
- Uses RAII locking for thread safety
- Computes per-variable pseudocosts with fallback to averages for uninitialized values
- Applies the reliability branching score formula (product of down/up estimates)
- Accumulates a conservative objective estimate using the minimum of up/down contributions
The algorithm aligns with standard MIP solver literature (Achterberg Sect. 6.4).
cpp/src/dual_simplex/diving_heuristics.hpp (1)
17-47: Clean diving heuristics API design.The header provides a well-organized interface for the four diving strategies from Achterberg's thesis:
- Line search diving (Sect. 9.2.4)
- Pseudocost diving (Sect. 9.2.5)
- Guided diving (Sect. 9.2.3)
- Coefficient diving (Sect. 9.2.1)
The
branch_variable_treturn type cleanly bundles the selected variable with its rounding direction.cpp/src/dual_simplex/diving_heuristics.cpp (3)
66-139: Correct pseudocost diving implementation with thread safety.The implementation correctly:
- Acquires mutex lock before accessing shared pseudocost data
- Uses fallback averages for uninitialized pseudocosts
- Applies the scoring formula from Achterberg Sect. 9.2.5
- Uses threshold-based direction selection (0.4 for root distance, 0.3/0.7 for fractionality)
The magic constants are standard heuristic values from MIP literature.
141-197: Correct guided diving implementation.The function properly implements Achterberg's guided diving (Sect. 9.2.3):
- Direction determined by proximity to incumbent solution
- Score weighted 5:1 in favor of the preferred direction
- Thread-safe pseudocost access
Note the same empty
fractionaledge case applies here.
229-273: Correct coefficient diving implementation.The function implements Achterberg's coefficient diving (Sect. 9.2.1):
- Selects variable with minimum constraint locks
- Prefers rounding direction that creates fewer constraint violations
- Uses fractionality as tie-breaker when locks are equal
The same empty
fractionaledge case consideration applies.cpp/src/dual_simplex/basis_solves.cpp (1)
613-622: Signature extension for bound-aware basis repair is complete and correctly integrated.The addition of
lowerandupperbound vectors enables correct variable status assignment during basis repair. All call sites have been updated to pass the bound vectors:
primal.cpp(1 call)crossover.cpp(3 calls)basis_updates.cpp(1 call)All invocations properly pass
lp.lowerandlp.upper(or locallower/upperfrom the calling context). The signature change aligns with the PR's goal of propagating bound information through solver phases.cpp/src/dual_simplex/node_queue.hpp (1)
60-101: Well-structured dual-heap design for unified node scheduling.The
node_queue_tclass effectively unifies best-first and diving heaps with shared entries, aligning with the PR objective to reduce memory consumption. The use ofshared_ptr<heap_entry_t>for cross-heap entry sharing is a clean approach.cpp/src/dual_simplex/phase2.cpp (3)
620-651: Signature change correctly separates squared and linear infeasibility.The updated
compute_initial_primal_infeasibilitiesnow properly computes both forms:
- Returns
primal_inf_squared(sum of squared infeasibilities)- Outputs
primal_inf(sum of linear infeasibilities) via referenceThis distinction is important for different algorithmic needs (squared for steepest-edge pricing weights, linear for feasibility checks). The initialization of
squared_infeasibilitiesvector at lines 631-632 ensures clean state.
2365-2373: Proper initialization of infeasibility tracking variables.The new code correctly initializes
primal_infeasibilityandprimal_infeasibility_squaredseparately, withcompute_initial_primal_infeasibilitiespopulating both values. This ensures clean state before the main iteration loop.
2248-2251: Allrefactor_basiscall sites are consistent with the function signature.Verification confirms that all three call sites in phase2.cpp (lines 2248, 2910–2911, and 2917–2918) correctly include the
lp.lowerandlp.upperbound parameters matching the function signature in basis_updates.hpp and basis_updates.cpp.cpp/src/dual_simplex/branch_and_bound.hpp (4)
56-67: Clear documentation and well-structured enum for diving strategies.The
bnb_thread_type_tenum provides clear categorization with references to the Achterberg dissertation sections. This aligns well with the PR objective of implementing multiple diving heuristics.
75-86: Thread-safe statistics structure with atomic counters.The
bnb_stats_tstruct appropriately usesomp_atomic_tfor counters that may be updated from multiple threads. Thelast_logandnodes_since_last_logmembers with the comment about main-thread-only usage help clarify the intended access pattern.
182-184: Unified node queue replaces separate heap structures.The replacement of the previous heap-based structures with
node_queue_taligns with the PR objective to "unify diving and best-first heaps into a single node_queue object." This should help reduce memory consumption as mentioned in the PR description.
261-266:variable_selectionreturn type and parameters are well-designed.Returning
branch_variable_t<i_t>which includes both the variable index and rounding direction is cleaner than having multiple output parameters. The additionallogger_t& logparameter enables per-call logging control.cpp/src/dual_simplex/branch_and_bound.cpp (9)
558-577: Martin's criteria implementation for rounding direction.The
martin_criteriafunction correctly implements the rounding heuristic based on distance from the root relaxation value. The epsilon tolerance (1e-6) for tie-breaking is reasonable.
579-619: Clean dispatch pattern for variable selection strategies.The
variable_selectionmethod uses a clear switch statement to dispatch to different diving heuristics. The objective estimate calculation is correctly scoped to only the EXPLORATION case where it's needed for heap ordering.
640-643: Appropriate fallback from GUIDED_DIVING to PSEUDOCOST_DIVING.When there's no incumbent solution (
upper_bound == inf), guided diving cannot function since it relies on the incumbent for guidance. Falling back to pseudocost diving is a sensible default.
656-661: Iteration budget enforcement for diving threads.The iteration limit for diving threads is computed as a factor of total B&B LP iterations minus iterations already consumed by this dive. This prevents diving from consuming too many iterations.
However, if
stats.total_lp_itersis close tomax_iter, this could returnITERATION_LIMITvery early. Verify that theiteration_limit_factorsetting provides sufficient budget for meaningful dives.
1143-1219: Deep diving implementation with backtrack limit.The
dive_frommethod implements the diving heuristics with proper backtracking control. Key observations:
- Lines 1163-1166: Local
dive_statsproperly tracks per-dive statistics- Lines 1179-1180: Time and node limits checked within the dive loop
- Lines 1215-1218: Backtrack limit enforces shallow backtracking by removing nodes from the back of the stack when depth difference exceeds the limit
The algorithm correctly prioritizes depth-first exploration while limiting the breadth of backtracking.
1248-1255: Bounds propagation for diving thread efficiency.The code correctly retrieves variable bounds from the node and applies bounds strengthening before diving. The
reset_starting_boundsflag ensures bounds are properly reset when starting from a new node.
1518-1535: Diving strategy configuration is flexible and well-structured.The diving strategies are populated based on settings, allowing users to disable specific strategies. The
reserve(4)call is appropriate given the four possible strategies.
251-260: Improved lower bound computation uses node_queue.The
get_lower_boundmethod correctly combines:
lower_bound_ceiling_(for numerical issue tracking)node_queue.get_lower_bound()(heap minimum)local_lower_bounds_(per-thread tracking)This provides an accurate global lower bound across all active exploration paths.
1354-1354: Row-major matrix initialization for bounds strengthening.Converting the constraint matrix to row-major format (
Arow_) enables efficient row-based access during bounds strengthening. This is initialized once at the start ofsolve(), avoiding repeated conversions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@cpp/src/dual_simplex/branch_and_bound.cpp`:
- Around line 592-599: The fallback for GUIDED_DIVING is inverted: in the block
checking get_upper_bound() == inf and type == bnb_worker_type_t::GUIDED_DIVING,
flip the branches using settings_.diving_settings.with_pseudocost_diving so that
when with_pseudocost_diving is true you set type =
bnb_worker_type_t::PSEUDOCOST_DIVING, otherwise set type =
bnb_worker_type_t::COEFFICIENT_DIVING; keep the same condition guards
(get_upper_bound(), inf, GUIDED_DIVING) and only swap the assignments.
In `@cpp/src/dual_simplex/node_queue.hpp`:
- Around line 120-145: pop_best_first() and pop_diving() perform unsynchronized
accesses to the heaps and rely on callers using lock()/unlock(), which is
error-prone and breaks RAII; make these methods take the internal lock like
push() by using std::lock_guard<std::mutex> inside pop_best_first() and
pop_diving(), remove or deprecate the public lock()/unlock() (or make them
private) to restore encapsulation, and ensure pop_diving() still loops correctly
under the lock so it safely checks diving_heap.empty() and pops entries while
holding the mutex.
- Around line 165-169: bfs_top() is dead or unsafe: either delete the bfs_top()
method entirely if it’s not part of a public API, or change its contract to
avoid returning a raw pointer after releasing the lock; for example, return
std::optional<mip_node_t<i_t, f_t>*> (or better, return a value/handle following
the existing pop_best_first()/pop_diving() patterns) and ensure access to
best_first_heap and mutex is performed while the lock is held so no
use-after-free can occur; update documentation/comments if you keep it for
external use.
♻️ Duplicate comments (6)
cpp/src/mip/diversity/lns/rins.cu (2)
259-268: Configuration updates align with the new diving_settings structure.The migration to the nested
diving_settingscontainer and the renaming fromnum_diving_threads/num_bfs_threadstonum_diving_workers/num_bfs_workersis consistent with the broader PR refactoring.The comment at lines 263-264 documents the intent to restrict RINS to guided diving. As noted in a previous review, consider explicitly setting
with_guided_diving = true(or adding a brief comment) to make it clear that guided diving is intentionally enabled by default when the other heuristics are disabled.
269-274: Data race onrins_solution_queueremains unaddressed.With
num_threads = 2(line 260), thesolution_callbackmay be invoked concurrently from multiple threads. Thepush_backcall on line 273 is not thread-safe and can cause memory corruption.This issue was flagged in a previous review. Please add synchronization (e.g.,
std::mutexwithstd::lock_guard) around thepush_backcall.cpp/src/dual_simplex/branch_and_bound.cpp (3)
286-304: Potential divide-by-zero inreport().Line 292 computes
iter_node = exploration_stats_.total_lp_iters / nodes_exploredwithout checking ifnodes_exploredis zero. This could cause undefined behavior ifreport()is called before any nodes are explored.🐛 Proposed fix
i_t nodes_explored = exploration_stats_.nodes_explored; i_t nodes_unexplored = exploration_stats_.nodes_unexplored; f_t user_obj = compute_user_objective(original_lp_, obj); f_t user_lower = compute_user_objective(original_lp_, lower_bound); - f_t iter_node = exploration_stats_.total_lp_iters / nodes_explored; + f_t iter_node = nodes_explored > 0 + ? static_cast<f_t>(exploration_stats_.total_lp_iters) / nodes_explored + : 0.0;
623-626: Default case returns invalidbranch_var = -1.The default case returns
{-1, rounding_direction_t::NONE}. Since this function's result is used to indexleaf_solution.x[branch_var](line 815), a negative index would cause out-of-bounds memory access. While the default case shouldn't normally be reached, defensive programming suggests either asserting or handling this case at the call site.
797-803: Assert won't protect in release builds.The
assert(branch_var >= 0)at line 801 only fires in debug builds. Ifvariable_selectionever returns -1 in production (e.g., due to the default case or an edge case), the subsequent array access at line 815 would cause undefined behavior. Consider adding a runtime check that returnsNO_CHILDRENor logs an error.🛡️ Suggested defensive check
auto [branch_var, round_dir] = variable_selection(node_ptr, leaf_fractional, leaf_solution.x, thread_type); assert(leaf_vstatus.size() == leaf_problem.num_cols); - assert(branch_var >= 0); - assert(round_dir != rounding_direction_t::NONE); + if (branch_var < 0 || round_dir == rounding_direction_t::NONE) { + log.printf("Variable selection failed for node %d\n", node_ptr->node_id); + search_tree.update(node_ptr, node_status_t::NUMERICAL); + return node_solve_info_t::NUMERICAL; + }cpp/src/dual_simplex/diving_heuristics.cpp (1)
200-228: Variable lock calculation doesn't implement correct lock semantics.The current implementation simply counts the number of nonzeros in each column, making
up_locks[j] == down_locks[j]for all variables. This defeats the purpose of coefficient diving, which should select variables based on asymmetric locks.Per Achterberg Definition 3.3, a variable has an "up lock" in a constraint if increasing it could make the constraint infeasible (depends on coefficient sign and constraint sense), and similarly for "down lock". The implementation should consider:
- The sign of the coefficient
val- The row sense (equality, ≤, or ≥)
As a result,
coefficient_divingwill always use fractionality as the tiebreaker (lines 262-265) sinceup_lock == down_lockalways.📝 Expected behavior sketch
for (i_t p = start; p < end; ++p) { f_t val = lp_problem.A.x[p]; i_t i = lp_problem.A.i[p]; // Need row sense information to determine lock direction // If row is <= constraint: // val > 0 → up_lock++ // val < 0 → down_lock++ // If row is >= constraint: // val > 0 → down_lock++ // val < 0 → up_lock++ // If row is == constraint: // up_lock++, down_lock++ }
🧹 Nitpick comments (3)
cpp/src/dual_simplex/simplex_solver_settings.hpp (1)
23-36: Well-structured diving heuristics settings.The new
diving_heuristics_settings_tstruct cleanly encapsulates all diving-related configuration. The defaults align with Achterberg (2007) recommendations.One minor suggestion from prior feedback: consider renaming
node_limittodiving_node_limitto avoid confusion with the user-facingnode_limitinsimplex_solver_settings_t(line 104).cpp/src/dual_simplex/node_queue.hpp (1)
147-157: Minor: Potential narrowing conversion fromsize_ttoi_t.
heap_t::size()returnssize_t, butdiving_queue_size()andbest_first_queue_size()returni_t. Ifi_tisint, this could truncate on very large queues (unlikely but worth noting).cpp/src/dual_simplex/branch_and_bound.cpp (1)
877-893: Minor TOCTOU in logging condition.The logging condition checks
exploration_stats_.nodes_since_last_logandexploration_stats_.last_log, then resets them outside a single atomic block. Multiple threads could potentially pass the condition simultaneously. However, since this only affects log output frequency (worst case: duplicate log lines), the impact is minimal.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cucpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/mip/local_search/rounding/simple_rounding_kernels.cuh
✅ Files skipped from review due to trivial changes (1)
- cpp/src/mip/local_search/rounding/simple_rounding_kernels.cuh
🚧 Files skipped from review as they are similar to previous changes (1)
- cpp/src/mip/diversity/recombiners/sub_mip.cuh
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{cu,cuh,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh,cpp,hpp,h}: Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Check that hard-coded GPU de...
Files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cpp
**/*.{h,hpp,py}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Verify C API does not break ABI stability (no struct layout changes, field reordering); maintain backward compatibility in Python and server APIs with deprecation warnings
Files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/branch_and_bound.hpp
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cpp,hpp,h}: Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Validate input sanitization to prevent buffer overflows and resource exhaustion attacks; avoid unsafe deserialization of problem files
Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cpp
**/*.{cu,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cpp
**/*.{cu,cuh}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh}: Every CUDA kernel launch and memory operation must have error checking with CUDA_CHECK or equivalent verification
Avoid reinventing functionality already available in Thrust, CCCL, or RMM libraries; prefer standard library utilities over custom implementations
Files:
cpp/src/mip/diversity/lns/rins.cu
**/*.cu
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.cu: Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Detect inefficient GPU kernel launches with low occupancy or poor memory access patterns; optimize for coalesced memory access and minimize warp divergence in hot paths
Files:
cpp/src/mip/diversity/lns/rins.cu
🧠 Learnings (27)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/pseudo_costs.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check that hard-coded GPU device IDs and resource limits are made configurable; abstract multi-backend support for different CUDA versions
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/pseudo_costs.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/pseudo_costs.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/pseudo_costs.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cpp,hpp,h} : Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/pseudo_costs.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify error propagation from CUDA to user-facing APIs is complete; ensure CUDA errors are caught and mapped to meaningful user error codes
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/pseudo_costs.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.cu : Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/pseudo_costs.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/pseudo_costs.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cu
📚 Learning: 2025-12-04T20:09:09.264Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 602
File: cpp/src/linear_programming/solve.cu:732-742
Timestamp: 2025-12-04T20:09:09.264Z
Learning: In cpp/src/linear_programming/solve.cu, the barrier solver does not currently return INFEASIBLE or UNBOUNDED status. It only returns OPTIMAL, TIME_LIMIT, NUMERICAL_ISSUES, or CONCURRENT_LIMIT.
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2025-12-04T04:11:12.640Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/src/dual_simplex/scaling.cpp:68-76
Timestamp: 2025-12-04T04:11:12.640Z
Learning: In the cuOPT dual simplex solver, CSR/CSC matrices (including the quadratic objective matrix Q) are required to have valid dimensions and indices by construction. Runtime bounds checking in performance-critical paths like matrix scaling is avoided to prevent slowdowns. Validation is performed via debug-only check_matrix() calls wrapped in `#ifdef` CHECK_MATRIX.
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/branch_and_bound.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/branch_and_bound.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cpp,hpp,h} : Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Applied to files:
cpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Applied to files:
cpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/diving_heuristics.hppcpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Ensure test isolation: prevent GPU state, cached memory, and global variables from leaking between test cases; verify each test independently initializes its environment
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/pseudo_costs.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Test with free variables, singleton problems, and extreme problem dimensions near resource limits to validate edge case handling
Applied to files:
cpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh} : Avoid reinventing functionality already available in Thrust, CCCL, or RMM libraries; prefer standard library utilities over custom implementations
Applied to files:
cpp/src/dual_simplex/diving_heuristics.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/diving_heuristics.cpp
📚 Learning: 2026-01-14T00:38:33.700Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 746
File: cpp/src/dual_simplex/barrier.cu:549-552
Timestamp: 2026-01-14T00:38:33.700Z
Learning: In cpp/src/dual_simplex/barrier.cu's form_adat() method within the barrier solver, the raft::copy from d_original_A_values to device_AD.x is necessary and must not be removed. This copy restores the original matrix A values before they are scaled by the diagonal matrix D (via d_inv_diag_prime). Since D changes between iterations and the scaling mutates device_AD.x, copying the original values each iteration is required for correctness.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.hpp
📚 Learning: 2025-10-22T14:25:22.899Z
Learnt from: aliceb-nv
Repo: NVIDIA/cuopt PR: 527
File: cpp/src/mip/diversity/lns/rins.cu:167-175
Timestamp: 2025-10-22T14:25:22.899Z
Learning: In MIP (Mixed Integer Programming) problems in the cuOPT codebase, `n_integer_vars == 0` is impossible by definition—MIP problems must have at least one integer variable. If there are no integer variables, it would be a pure Linear Programming (LP) problem, not a MIP problem.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.hpp
🧬 Code graph analysis (4)
cpp/src/dual_simplex/pseudo_costs.cpp (1)
cpp/src/dual_simplex/pseudo_costs.hpp (4)
fractional(46-49)fractional(51-53)fractional(55-56)num_initialized_down(41-44)
cpp/src/dual_simplex/branch_and_bound.cpp (3)
cpp/src/dual_simplex/branch_and_bound.hpp (6)
symbol(207-207)obj(206-206)solution(116-116)solution(134-134)solution(210-210)node(224-224)cpp/src/dual_simplex/diving_heuristics.hpp (3)
line_search_diving(24-27)pseudocost_diving(30-34)guided_diving(37-41)cpp/src/dual_simplex/node_queue.hpp (4)
node(28-32)node(28-28)node(34-38)node(34-34)
cpp/src/dual_simplex/branch_and_bound.hpp (1)
cpp/src/dual_simplex/node_queue.hpp (4)
node(28-32)node(28-28)node(34-38)node(34-34)
cpp/src/dual_simplex/diving_heuristics.cpp (1)
cpp/src/dual_simplex/pseudo_costs.hpp (4)
fractional(46-49)fractional(51-53)fractional(55-56)num_initialized_down(41-44)
🪛 Clang (14.0.6)
cpp/src/dual_simplex/node_queue.hpp
[error] 8-8: 'algorithm' file not found
(clang-diagnostic-error)
cpp/src/dual_simplex/diving_heuristics.hpp
[error] 10-10: 'dual_simplex/basis_updates.hpp' file not found
(clang-diagnostic-error)
cpp/src/dual_simplex/branch_and_bound.hpp
[error] 10-10: 'dual_simplex/diving_heuristics.hpp' file not found
(clang-diagnostic-error)
cpp/src/dual_simplex/diving_heuristics.cpp
[error] 8-8: 'dual_simplex/diving_heuristics.hpp' file not found
(clang-diagnostic-error)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-sh-client / 13.1.0, 3.10, amd64, rockylinux8
🔇 Additional comments (23)
cpp/src/dual_simplex/simplex_solver_settings.hpp (2)
89-97: LGTM - Worker allocation logic is correct.The use of
std::maxensures at least 1 worker for both BFS and diving tasks, regardless of thread count. The allocation strategy (25% BFS, remainder diving) is reasonable for the workload distribution.
156-158: LGTM - Clean integration of diving settings.The
diving_settingsmember is properly typed and commented. The separation of diving configuration into its own struct improves modularity. Based on learnings, this aligns with the goal to reduce tight coupling between solver components.cpp/src/dual_simplex/pseudo_costs.hpp (1)
46-49: LGTM - New obj_estimate declaration.The function signature is appropriate for computing pseudocost-based objective estimates used in diving node prioritization. The implementation correctly uses mutex protection for thread safety.
cpp/src/dual_simplex/pseudo_costs.cpp (3)
202-214: LGTM - Proper RAII locking and correct frac calculation.The switch to
std::lock_guardeliminates the risk of forgotten unlocks. Thefraccalculation correctly handles both DOWN (distance to floor) and UP (distance to ceil) branches.
256-314: LGTM - Variable selection with proper synchronization.The
std::lock_guardensures thread-safe access. The product-based scoring formula correctly implements pseudocost branching as per Achterberg's variable selection strategy.
317-360: No changes needed. The implementation follows established branch-and-bound pseudo-cost practices. Thestd::max(..., eps)floor for negative pseudo-costs is intentional heuristic design (consistent with literature on objective estimation in MIP solvers), not a correctness issue. Lock contention is negligible given the minimal per-iteration compute cost in the fractional variable loop.cpp/src/dual_simplex/diving_heuristics.hpp (2)
17-21: LGTM - Clean branch_variable_t struct.Simple and effective struct for returning branching decisions from diving heuristics. The use of
rounding_direction_tfor direction is type-safe.
23-56: LGTM - Diving heuristic implementations follow Achterberg (2007) with proper initialization and state management.All four diving strategies are correctly implemented:
- Line search diving (§9.2.4): follows root-to-current solution direction, with fallback for degenerate cases
- Pseudocost diving (§9.2.5): uses pseudocost estimates with thread-safe mutex locking
- Guided diving (§9.2.3): moves toward incumbent solution, also thread-safe
- Coefficient diving (§9.2.1): uses variable locks with proper finite-bound detection
The
calculate_variable_lockshelper correctly documents its assumption (Ax = bformat). All functions initialize variables properly (branch_var=-1, round_dir=NONE) and have post-condition assertions. Functions are called only when fractional variables exist (guarded byif (leaf_num_fractional == 0)checks in branch_and_bound.cpp), ensuring safe vector access.cpp/src/dual_simplex/node_queue.hpp (2)
22-65: LGTM - Generic heap implementation.The
heap_tclass provides a clean wrapper around STL heap operations with direct buffer access. The use ofstd::optionalforpop()is idiomatic and handles empty heaps gracefully.Minor note: The
virtual ~heap_t() = defaultis unnecessary for a non-polymorphic class but harmless.
71-100: Good design - Shared heap entries for deduplication.The
heap_entry_tstores value snapshots (lower_bound,score) rather than reading from the node pointer during comparison. This prevents issues with nodes being modified/deleted after insertion. The sharedstd::shared_ptr<heap_entry_t>between both heaps enables efficient deduplication via the nullptr check.cpp/src/dual_simplex/branch_and_bound.hpp (4)
56-67: LGTM!The
bnb_worker_type_tenum properly categorizes the worker types with clear documentation referencing Achterberg (2007). The naming addresses the past reviewer feedback to renameEXPLORATIONtoBEST_FIRST.
75-86: LGTM!The
bnb_stats_tstruct properly uses atomic wrappers for thread-safe statistics tracking across concurrent B&B workers. The consolidation of timing, node counts, and logging metrics into a single struct improves maintainability.
144-154: LGTM!The new data members align with the PR objectives:
Arow_provides row-based matrix access for bounds strengtheningvar_up_locks_/var_down_locks_support the coefficient diving heuristic- Documentation references the Achterberg thesis appropriately
206-276: LGTM!The method signatures are well-designed for the new diving heuristics architecture. The use of
bnb_worker_type_tfor worker differentiation andbnb_stats_t&for statistics tracking enables clean separation of concerns between best-first search and diving threads.cpp/src/dual_simplex/branch_and_bound.cpp (4)
197-207: LGTM!The
feasible_solution_symbolfunction cleanly maps worker types to log symbols. Usingcharinstead ofstd::stringaddresses the past reviewer feedback.
1125-1206: LGTM!The
dive_fromfunction is well-designed with proper isolation:
- Local
dive_treeanddive_statsprevent race conditions with global state- Respects node and iteration limits for controlled exploration
- Backtrack limit at lines 1200-1204 prevents unbounded stack growth
1208-1262: LGTM!The
diving_threadfunction properly isolates diving exploration:
- Uses
detach_copy()to create independent node copies for diving- Proper mutex protection around
node_queue_operations- Local state (bounds, presolver) prevents interference between diving threads
1348-1372: LGTM!The diving strategy setup correctly:
- Builds the strategy vector based on settings
- Calculates variable locks only when coefficient diving is enabled
- Includes the warning with proper newline (addressing past review)
- The empty check at line 1558 prevents divide-by-zero in round-robin assignment
cpp/src/dual_simplex/diving_heuristics.cpp (5)
13-65: LGTM!The
line_search_divingimplementation correctly follows Achterberg §9.2.4:
- Score computation
f/dselects variables moving toward integer values- The epsilon tolerance ensures no division by near-zero
- Fallback to first fractional variable handles edge case where solution equals root
67-140: LGTM!The
pseudocost_divingimplementation is well-designed:
- Proper mutex protection for pseudo-cost access
- Fallback to averages when per-variable costs aren't initialized
- The scoring formula combines fractionality and pseudocost information effectively
142-198: LGTM!The
guided_divingimplementation correctly guides variable rounding toward the incumbent solution. The 5:1 weighting ratio for preferred vs. non-preferred direction scores appropriately biases selection toward the incumbent.
230-281: LGTM, but effectiveness depends on lock calculation fix.The
coefficient_divinglogic is correct: it selects the variable with minimum locks and breaks ties using lock asymmetry and fractionality. The bound-based lock increment (lines 250-251) correctly treats finite bounds as additional constraints.However, the effectiveness of this heuristic depends on fixing
calculate_variable_locksto compute asymmetric lock counts.
283-311: LGTM!Template instantiations are complete for all diving heuristic functions with the
int/doublespecialization.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cpp/src/dual_simplex/branch_and_bound.cpp (1)
286-304: Potential divide-by-zero and integer truncation inreport().Line 292 divides by
nodes_exploredwhich can be zero whenreport()is called before any nodes are explored (e.g., during early ramp-up phase). Additionally, if both operands are integers, the division will truncate.🐛 Proposed fix
i_t nodes_explored = exploration_stats_.nodes_explored; i_t nodes_unexplored = exploration_stats_.nodes_unexplored; f_t user_obj = compute_user_objective(original_lp_, obj); f_t user_lower = compute_user_objective(original_lp_, lower_bound); - f_t iter_node = exploration_stats_.total_lp_iters / nodes_explored; + const i_t denom = nodes_explored > 0 ? nodes_explored : 1; + f_t iter_node = static_cast<f_t>(exploration_stats_.total_lp_iters) / denom; std::string user_gap = user_mip_gap<f_t>(user_obj, user_lower);
♻️ Duplicate comments (4)
cpp/src/dual_simplex/branch_and_bound.cpp (4)
870-893: Race conditions in ramp-up logging.Lines 873, 889-890 modify
exploration_stats_fields (nodes_unexplored,nodes_since_last_log,last_log) without synchronization in a parallel context (called via#pragma omp task).
659-664: Race condition readingexploration_stats_.total_lp_iterswithout synchronization.Line 660 reads
exploration_stats_.total_lp_iterswhich is concurrently modified by multiple threads (lines 743, 921, 1024). This unsynchronized read can return inconsistent values.Consider using atomic operations for
total_lp_itersor protecting accesses with the suggestedmutex_stats_.
750-752: Data race onexploration_stats_fields.These increments/decrements on
nodes_since_last_log,nodes_explored, andnodes_unexploredare performed without synchronization whenstatsrefers toexploration_stats_(in best-first threads). Multiple threads can modify these concurrently, causing lost updates.Consider using atomic operations for these counters or protecting them with a mutex.
797-812: Assert is insufficient protection for invalidbranch_var.Line 801 uses
assert(branch_var >= 0)which may be disabled in release builds. Ifvariable_selectionreturns-1(from the default case), the subsequent array accesses will cause undefined behavior.Replace the assert with a runtime check that handles the failure gracefully.
🐛 Proposed fix
auto [branch_var, round_dir] = variable_selection(node_ptr, leaf_fractional, leaf_solution.x, thread_type); assert(leaf_vstatus.size() == leaf_problem.num_cols); - assert(branch_var >= 0); - assert(round_dir != rounding_direction_t::NONE); + if (branch_var < 0 || round_dir == rounding_direction_t::NONE) { + search_tree.graphviz_node(log, node_ptr, "no branch variable", leaf_objective); + search_tree.update(node_ptr, node_status_t::NUMERICAL); + return node_solve_info_t::NUMERICAL; + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cpp/src/dual_simplex/branch_and_bound.cpp
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cu,cuh,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh,cpp,hpp,h}: Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Check that hard-coded GPU de...
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cpp,hpp,h}: Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Validate input sanitization to prevent buffer overflows and resource exhaustion attacks; avoid unsafe deserialization of problem files
Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cu,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Test with free variables, singleton problems, and extreme problem dimensions near resource limits to validate edge case handling
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cpp,hpp,h} : Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify error propagation from CUDA to user-facing APIs is complete; ensure CUDA errors are caught and mapped to meaningful user error codes
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cpp,hpp,h} : Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.cu : Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-12-04T20:09:09.264Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 602
File: cpp/src/linear_programming/solve.cu:732-742
Timestamp: 2025-12-04T20:09:09.264Z
Learning: In cpp/src/linear_programming/solve.cu, the barrier solver does not currently return INFEASIBLE or UNBOUNDED status. It only returns OPTIMAL, TIME_LIMIT, NUMERICAL_ISSUES, or CONCURRENT_LIMIT.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-12-04T04:11:12.640Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/src/dual_simplex/scaling.cpp:68-76
Timestamp: 2025-12-04T04:11:12.640Z
Learning: In the cuOPT dual simplex solver, CSR/CSC matrices (including the quadratic objective matrix Q) are required to have valid dimensions and indices by construction. Runtime bounds checking in performance-critical paths like matrix scaling is avoided to prevent slowdowns. Validation is performed via debug-only check_matrix() calls wrapped in `#ifdef` CHECK_MATRIX.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
🧬 Code graph analysis (1)
cpp/src/dual_simplex/branch_and_bound.cpp (2)
cpp/src/dual_simplex/branch_and_bound.hpp (12)
symbol(207-207)obj(206-206)node_ptr(258-270)node_ptr(273-276)solution(116-116)solution(134-134)solution(210-210)node(224-224)task_id(229-235)task_id(239-239)start_node(243-251)diving_type(255-255)cpp/src/dual_simplex/diving_heuristics.hpp (5)
coefficient_diving(51-56)line_search_diving(24-27)pseudocost_diving(30-34)guided_diving(37-41)calculate_variable_locks(46-48)
🔇 Additional comments (9)
cpp/src/dual_simplex/branch_and_bound.cpp (9)
197-207: LGTM!The helper function cleanly maps worker types to single-character symbols. The switch covers all diving variants uniformly with 'D' and provides a safe default.
217-239: LGTM!Constructor properly initializes the new
Arow_member with a placeholder, which is correctly populated viato_compressed_rowinsolve()before use.
558-577: LGTM!The Martin's criteria implementation correctly determines the rounding direction based on proximity to the root relaxation value, with appropriate epsilon tolerance for numerical stability.
1124-1206: LGTM!The
dive_fromimplementation correctly manages a local search tree and statistics, avoiding race conditions. The backtrack pruning logic properly limits exploration depth while maintaining the invariant that only reachable siblings remain in the stack.
1208-1262: LGTM!The
diving_threadcorrectly isolates its state by creating local problem copies and usingdetach_copyto obtain an independent node for diving. The queue access is properly synchronized with lock/unlock.
1348-1372: LGTM!The diving strategies initialization correctly populates the vector based on settings and includes appropriate warning when all heuristics are disabled. Variable locks are calculated lazily only when coefficient diving is enabled.
1568-1571: LGTM!The final lower bound calculation correctly handles both the case where unexplored nodes remain in the queue and the case where the queue is empty, falling back to the root's lower bound.
1535-1566: Thread allocation is correctly enforced by settings design.The settings constructor (simplex_solver_settings.hpp:88-96) explicitly ensures
num_diving_workers = std::max(num_threads - num_bfs_workers, 1), guaranteeing that total spawned tasks respect the thread pool. The parallel section correctly usestaskgroupto synchronize exploration ramp-up before spawning BFS and diving tasks. The empty check at line 1558 guards against accessingdiving_strategieswhen no strategies are available.
250-262: LGTM!The lower bound calculation correctly combines bounds from multiple sources (ceiling, node queue, local bounds). All sources use proper thread-safe synchronization:
lower_bound_ceiling_uses atomic loadnode_queue_.get_lower_bound()acquires a mutex viastd::lock_guardbefore accessing the heaplocal_lower_bounds_elements use atomic loadThe implementation is thread-safe and correct.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cpp/src/dual_simplex/branch_and_bound.cpp`:
- Around line 655-660: The assignment to lp_settings.iteration_limit from
floating f_t max_iter can overflow/UB when max_iter is huge or inf; instead
compute a safe integer limit: check that exploration_stats_.total_lp_iters and
settings_.diving_settings.iteration_limit_factor produce a finite max_iter (use
isfinite or equivalent), clamp max_iter to a safe max (e.g.,
numeric_limits<decltype(lp_settings.iteration_limit)>::max()), convert to the
integer type via a bounded cast, then subtract stats.total_lp_iters and ensure
the result is non-negative before assigning to lp_settings.iteration_limit and
returning node_solve_info_t::ITERATION_LIMIT if zero or negative.
♻️ Duplicate comments (3)
cpp/src/dual_simplex/branch_and_bound.cpp (3)
286-304: Potential divide-by-zero inreport()when no nodes have been explored yet.Line 292 divides by
nodes_exploredwhich could be 0 ifreport()is called early (e.g., fromadd_feasible_solution()before nodes are processed).
598-601: Missing validation ofbranch_varbefore array access in BEST_FIRST case.If
pc_.variable_selectionreturns -1 (e.g., for edge cases), the subsequentsolution[branch_var]androot_relax_soln_.x[branch_var]accesses would cause undefined behavior.
850-941: Race conditions onexploration_stats_persist in this parallel region.Multiple fields of
exploration_stats_(lines 869, 885-886, 917, 925) are accessed/modified without synchronization from parallel OpenMP tasks. This was flagged in a previous review.
🧹 Nitpick comments (1)
cpp/src/dual_simplex/branch_and_bound.cpp (1)
156-159: Consider removing unusedsgnfunction.This function appears to be unused in this file. If it's used elsewhere, consider moving it to a common utilities header to avoid duplication; otherwise, remove it to reduce code clutter.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cpp/src/dual_simplex/branch_and_bound.cpp
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cu,cuh,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh,cpp,hpp,h}: Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Check that hard-coded GPU de...
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cpp,hpp,h}: Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Validate input sanitization to prevent buffer overflows and resource exhaustion attacks; avoid unsafe deserialization of problem files
Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cu,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Test with free variables, singleton problems, and extreme problem dimensions near resource limits to validate edge case handling
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cpp,hpp,h} : Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify error propagation from CUDA to user-facing APIs is complete; ensure CUDA errors are caught and mapped to meaningful user error codes
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cpp,hpp,h} : Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.cu : Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Write tests validating numerical correctness of optimization results (not just 'runs without error'); test degenerate cases (infeasible, unbounded, empty, singleton problems)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-12-04T04:11:12.640Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/src/dual_simplex/scaling.cpp:68-76
Timestamp: 2025-12-04T04:11:12.640Z
Learning: In the cuOPT dual simplex solver, CSR/CSC matrices (including the quadratic objective matrix Q) are required to have valid dimensions and indices by construction. Runtime bounds checking in performance-critical paths like matrix scaling is avoided to prevent slowdowns. Validation is performed via debug-only check_matrix() calls wrapped in `#ifdef` CHECK_MATRIX.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-12-04T20:09:09.264Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 602
File: cpp/src/linear_programming/solve.cu:732-742
Timestamp: 2025-12-04T20:09:09.264Z
Learning: In cpp/src/linear_programming/solve.cu, the barrier solver does not currently return INFEASIBLE or UNBOUNDED status. It only returns OPTIMAL, TIME_LIMIT, NUMERICAL_ISSUES, or CONCURRENT_LIMIT.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, amd64, rockylinux8
🔇 Additional comments (8)
cpp/src/dual_simplex/branch_and_bound.cpp (8)
250-262: LGTM!The
get_lower_boundimplementation correctly combines multiple sources (atomic ceiling, node queue, per-thread local bounds) to compute the global lower bound with proper thread-safe access patterns.
592-595: LGTM! Simplified GUIDED_DIVING fallback logic.The simplified fallback that unconditionally uses
PSEUDOCOST_DIVINGwhen there's no incumbent is cleaner and avoids the previously flagged inverted logic issue.
1120-1202: LGTM! Well-isolated diving implementation.The use of local
dive_statsand a separatedive_treeeffectively isolates diving operations from the main search tree and avoids race conditions withexploration_stats_. The backtrack limit enforcement at lines 1196-1200 correctly bounds the stack depth.
1204-1258: LGTM! Thread-safe diving thread implementation.The lock/unlock pattern around
node_queue_operations is correct, anddetach_copy()at line 1237 properly creates an independent copy before releasing the lock, preventing use-after-free issues.
1346-1368: LGTM! Clean diving strategy setup.The conditional construction of
diving_strategiesbased on settings is well-organized, andcalculate_variable_locksis only called when coefficient diving is enabled, avoiding unnecessary computation. The warning message now correctly includes a newline.
1531-1562: LGTM! Well-structured parallel execution.The
#pragma omp taskgroupensures ramp-up completes before best-first and diving threads compete for nodes. The empty check at line 1554 correctly guards against the division-by-zero issue in round-robin strategy assignment.
1053-1118: LGTM! Proper queue synchronization in best-first thread.The lock/unlock pattern around
node_queue_operations is correct, and the use ofactive_subtrees_for termination detection prevents premature exit when nodes are still being processed.
1000-1007: Verify break behavior doesn't lose remaining stack nodes.When
time_limitornode_limitis reached (lines 1002, 1006), the loop breaks immediately. Any nodes remaining in the localstackare abandoned. This is acceptable for termination, but consider whether those nodes should be pushed back tonode_queue_for later reporting ofnodes_unexplored.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
chris-maes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments, but overall looks good. Fine to leave variable selection as is for now.
It would be good to review the benchmark before merging.
chris-maes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments, but overall looks good. Fine to leave variable selection as is for now.
It would be good to review the benchmark before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cpp/src/mip/diversity/recombiners/sub_mip.cuh`:
- Around line 106-117: The branch-and-bound callback assigned to
branch_and_bound_settings.solution_callback can run concurrently when
num_bfs_workers > 1, causing unsynchronized push_back into solution_vector and
data races; protect the critical section by serializing access: add a mutex
(e.g., a member std::mutex) and lock it inside the lambda before calling
this->solution_callback or pushing into solution_vector, or alternatively force
single-threaded callbacks by setting num_bfs_workers to 1; update the lambda
defined in solution_callback to acquire the lock, call
this->solution_callback(solution, objective), and release the lock to prevent
concurrent modifications.
♻️ Duplicate comments (6)
cpp/src/mip/diversity/lns/rins.cu (1)
259-274: Guardrins_solution_queuein the multi-threaded callback.With
num_threads = 2, the callback may run concurrently;rins_solution_queue.push_backis not synchronized and can corrupt memory.🔧 Suggested fix (mutex-protected queue)
- branch_and_bound_settings.solution_callback = [&rins_solution_queue](std::vector<f_t>& solution, - f_t objective) { - rins_solution_queue.push_back(solution); - }; + branch_and_bound_settings.solution_callback = + [&rins_solution_queue, &rins_solution_queue_mutex](std::vector<f_t>& solution, f_t objective) { + std::lock_guard<std::mutex> lock(rins_solution_queue_mutex); + rins_solution_queue.push_back(solution); + };Additional change needed outside this hunk:
- std::vector<std::vector<f_t>> rins_solution_queue; + std::vector<std::vector<f_t>> rins_solution_queue; + std::mutex rins_solution_queue_mutex;cpp/src/dual_simplex/node_queue.hpp (2)
115-135: Locking remains caller-managed for pop_ APIs.*
This still relies on external lock()/unlock() and can be misused; the earlier concern stands.
155-159: bfs_top still exposes a raw pointer without lifetime guarantees.
Same UAF risk as previously noted if callers use the pointer after concurrent mutations.cpp/src/dual_simplex/branch_and_bound.cpp (3)
269-277: report() still risks divide-by-zero and format mismatch.
The earlier concern (nodes_explored == 0 and %lu vs i_t) remains.
626-631: Iteration-limit conversion still risks overflow/UB.
The float→integer assignment and potential infinity/overflow remain as previously flagged.
1231-1245: Potential race: get_variable_bounds on nodes that may be deleted.
The prior use-after-free risk during concurrent tree updates still applies here.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
cpp/src/dual_simplex/branch_and_bound.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/pseudo_costs.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/mip/diversity/lns/rins.cucpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/utilities/omp_helpers.hpp
🚧 Files skipped from review as they are similar to previous changes (1)
- cpp/src/dual_simplex/pseudo_costs.hpp
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{cu,cuh,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh,cpp,hpp,h}: Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Check that hard-coded GPU de...
Files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/utilities/omp_helpers.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cpp,hpp,h}: Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Validate input sanitization to prevent buffer overflows and resource exhaustion attacks; avoid unsafe deserialization of problem files
Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/utilities/omp_helpers.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cu,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/utilities/omp_helpers.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cu,cuh}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh}: Every CUDA kernel launch and memory operation must have error checking with CUDA_CHECK or equivalent verification
Avoid reinventing functionality already available in Thrust, CCCL, or RMM libraries; prefer standard library utilities over custom implementations
Files:
cpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/mip/diversity/lns/rins.cu
**/*.{h,hpp,py}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Verify C API does not break ABI stability (no struct layout changes, field reordering); maintain backward compatibility in Python and server APIs with deprecation warnings
Files:
cpp/src/utilities/omp_helpers.hppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hpp
**/*.cu
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.cu: Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Detect inefficient GPU kernel launches with low occupancy or poor memory access patterns; optimize for coalesced memory access and minimize warp divergence in hot paths
Files:
cpp/src/mip/diversity/lns/rins.cu
🧠 Learnings (27)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2026-01-15T20:41:51.616Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 697
File: cpp/src/dual_simplex/diving_heuristics.cpp:200-228
Timestamp: 2026-01-15T20:41:51.616Z
Learning: In cpp/src/dual_simplex/diving_heuristics.cpp, the calculate_variable_locks function incorrectly indexes lp_problem.lower[nz_row] and lp_problem.upper[nz_row] where nz_row is a row index (0 to num_rows-1), but lower and upper are variable bounds of size num_cols indexed by column. This causes semantic errors and potential out-of-bounds access when num_rows > num_cols. Since lp_problem_t constraints are always equalities (A*x = rhs) after presolve, lock counting should treat each nonzero as contributing equally to both up_lock and down_lock, rather than checking constraint bounds.
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cpp,hpp,h} : Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check that hard-coded GPU device IDs and resource limits are made configurable; abstract multi-backend support for different CUDA versions
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/utilities/omp_helpers.hppcpp/src/dual_simplex/simplex_solver_settings.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/utilities/omp_helpers.hppcpp/src/dual_simplex/simplex_solver_settings.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/utilities/omp_helpers.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cpp,hpp,h} : Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/utilities/omp_helpers.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify error propagation from CUDA to user-facing APIs is complete; ensure CUDA errors are caught and mapped to meaningful user error codes
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/utilities/omp_helpers.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/utilities/omp_helpers.hppcpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.cu : Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/utilities/omp_helpers.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/utilities/omp_helpers.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/utilities/omp_helpers.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/simplex_solver_settings.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Applied to files:
cpp/src/dual_simplex/pseudo_costs.cppcpp/src/mip/diversity/recombiners/sub_mip.cuhcpp/src/utilities/omp_helpers.hppcpp/src/mip/diversity/lns/rins.cucpp/src/dual_simplex/simplex_solver_settings.hpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Applied to files:
cpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Test with free variables, singleton problems, and extreme problem dimensions near resource limits to validate edge case handling
Applied to files:
cpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/node_queue.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Write tests validating numerical correctness of optimization results (not just 'runs without error'); test degenerate cases (infeasible, unbounded, empty, singleton problems)
Applied to files:
cpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-12-04T20:09:09.264Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 602
File: cpp/src/linear_programming/solve.cu:732-742
Timestamp: 2025-12-04T20:09:09.264Z
Learning: In cpp/src/linear_programming/solve.cu, the barrier solver does not currently return INFEASIBLE or UNBOUNDED status. It only returns OPTIMAL, TIME_LIMIT, NUMERICAL_ISSUES, or CONCURRENT_LIMIT.
Applied to files:
cpp/src/dual_simplex/diving_heuristics.cppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-12-04T04:11:12.640Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/src/dual_simplex/scaling.cpp:68-76
Timestamp: 2025-12-04T04:11:12.640Z
Learning: In the cuOPT dual simplex solver, CSR/CSC matrices (including the quadratic objective matrix Q) are required to have valid dimensions and indices by construction. Runtime bounds checking in performance-critical paths like matrix scaling is avoided to prevent slowdowns. Validation is performed via debug-only check_matrix() calls wrapped in `#ifdef` CHECK_MATRIX.
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Applied to files:
cpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/dual_simplex/branch_and_bound.hppcpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2026-01-14T00:38:38.038Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 746
File: cpp/src/dual_simplex/barrier.cu:549-552
Timestamp: 2026-01-14T00:38:38.038Z
Learning: In cpp/src/dual_simplex/barrier.cu's form_adat() method within the barrier solver, the raft::copy from d_original_A_values to device_AD.x is necessary and must not be removed. This copy restores the original matrix A values before they are scaled by the diagonal matrix D (via d_inv_diag_prime). Since D changes between iterations and the scaling mutates device_AD.x, copying the original values each iteration is required for correctness.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.hpp
📚 Learning: 2025-10-22T14:25:22.899Z
Learnt from: aliceb-nv
Repo: NVIDIA/cuopt PR: 527
File: cpp/src/mip/diversity/lns/rins.cu:167-175
Timestamp: 2025-10-22T14:25:22.899Z
Learning: In MIP (Mixed Integer Programming) problems in the cuOPT codebase, `n_integer_vars == 0` is impossible by definition—MIP problems must have at least one integer variable. If there are no integer variables, it would be a pure Linear Programming (LP) problem, not a MIP problem.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.hpp
🧬 Code graph analysis (5)
cpp/src/dual_simplex/pseudo_costs.cpp (1)
cpp/src/dual_simplex/pseudo_costs.hpp (4)
fractional(46-49)fractional(51-53)fractional(55-56)num_initialized_down(41-44)
cpp/src/mip/diversity/lns/rins.cu (2)
cpp/src/dual_simplex/branch_and_bound.hpp (3)
solution(98-98)solution(115-115)solution(192-192)cpp/src/mip/solver.cu (6)
solution(64-68)solution(64-64)solution(70-75)solution(70-72)solution(77-80)solution(77-77)
cpp/src/dual_simplex/diving_heuristics.cpp (1)
cpp/src/dual_simplex/pseudo_costs.hpp (4)
fractional(46-49)fractional(51-53)fractional(55-56)num_initialized_down(41-44)
cpp/src/dual_simplex/node_queue.hpp (3)
cpp/src/dual_simplex/branch_and_bound.hpp (4)
node(206-206)node_ptr(240-252)node_ptr(257-263)node_ptr(266-269)cpp/src/dual_simplex/pseudo_costs.hpp (1)
node_ptr(31-31)cpp/src/dual_simplex/mip_node.hpp (2)
node_ptr(277-283)node_ptr(277-277)
cpp/src/dual_simplex/branch_and_bound.hpp (1)
cpp/src/dual_simplex/node_queue.hpp (4)
node(28-32)node(28-28)node(34-38)node(34-34)
🪛 Clang (14.0.6)
cpp/src/dual_simplex/diving_heuristics.cpp
[error] 8-8: 'dual_simplex/diving_heuristics.hpp' file not found
(clang-diagnostic-error)
cpp/src/dual_simplex/node_queue.hpp
[error] 8-8: 'algorithm' file not found
(clang-diagnostic-error)
cpp/src/dual_simplex/branch_and_bound.hpp
[error] 10-10: 'dual_simplex/diving_heuristics.hpp' file not found
(clang-diagnostic-error)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.13, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.10, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, amd64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.12, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.1.0, 3.11, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-sh-client / 13.1.0, 3.10, amd64, rockylinux8
🔇 Additional comments (15)
cpp/src/mip/diversity/recombiners/sub_mip.cuh (1)
3-3: No review needed for copyright-only update.cpp/src/dual_simplex/simplex_solver_settings.hpp (6)
3-3: No review needed for copyright-only update.
16-16: No review needed for this include-only change.
23-37: Diving heuristics settings struct looks good.Nice consolidation with clear -1/0/1 semantics for heuristics.
90-90: Sane default for BFS worker allocation.Keeps at least one worker available by default.
97-97: Default diving worker count initialization looks consistent.
157-160: Clear separation of BFS workers and diving settings.cpp/src/utilities/omp_helpers.hpp (2)
3-3: No review needed for copyright-only update.
56-56: Const-correctload()is a good cleanup.cpp/src/dual_simplex/pseudo_costs.cpp (5)
3-3: No review needed for copyright-only update.
151-152: Resetting strong-branch arrays viaassignlooks good.
202-202: Thread-safe pseudo-cost updates are good.
260-260: Locking around variable selection is good.
317-359: Objective estimate helper looks good.Nice addition with clean averaging fallback.
cpp/src/dual_simplex/diving_heuristics.cpp (1)
200-219: Lock counting now matches equality-only constraint model.
Counting nonzeros for both up/down locks avoids the earlier row/col bound mix-up and fits the presolvedAx = brepresentation. Based on learnings, this is the right fix.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cpp/src/dual_simplex/branch_and_bound.cpp`:
- Around line 581-610: The fallback that unconditionally sets type =
bnb_worker_type_t::PSEUDOCOST_DIVING when upper_bound_ == inf ignores diving
flags; change that logic to first check
settings_.diving_settings.pseudocost_diving != 0 and only pick PSEUDOCOST_DIVING
if enabled, otherwise pick the first enabled diving strategy used elsewhere
(e.g., COEFFICIENT_DIVING, LINE_SEARCH_DIVING, PSEUDOCOST_DIVING order or the
same order used in the existing fallback pattern) and assign type accordingly;
also fix the log call in the default case to cast the enum class when printing
(use static_cast<int>(type)) so bnb_worker_type_t is logged correctly.
♻️ Duplicate comments (4)
cpp/src/dual_simplex/branch_and_bound.cpp (4)
283-300: Guard iter/node against divide-by-zero and truncation.At Line 289,
nodes_exploredcan be 0 early in the run, which will divide by zero; the current expression also truncates due to integer division. Also ensure theprintfspec fornodes_unexploredmatchesi_t.🔧 Suggested fix
- f_t iter_node = exploration_stats_.total_lp_iters / nodes_explored; + const i_t denom = nodes_explored > 0 ? nodes_explored : 1; + f_t iter_node = static_cast<f_t>(exploration_stats_.total_lp_iters) / denom; @@ - settings_.log.printf("%c %10d %10lu %+13.6e %+10.6e %6d %7.1e %s %9.2f\n", + settings_.log.printf("%c %10d %10d %+13.6e %+10.6e %6d %7.1e %s %9.2f\n", symbol, nodes_explored, nodes_unexplored,
855-1033: Protect exploration_stats_ updates from data races.
exploration_stats_is updated by multiple OpenMP tasks (e.g., nodes explored/unexplored, log counters) without synchronization, which is a data race. Please guard with a mutex or atomics.
As per coding guidelines, ensure race conditions are absent in multi-threaded code.
640-646: Clamp iteration limits to avoid overflow/UB.At Line 643–645,
factor * bnb_lp_itersis cast intoi_tand may overflow/UB when large or non-finite. Please clamp and cast safely.🔧 Suggested fix
- i_t bnb_lp_iters = exploration_stats_.total_lp_iters; - f_t factor = settings_.diving_settings.iteration_limit_factor; - i_t max_iter = factor * bnb_lp_iters; - lp_settings.iteration_limit = max_iter - stats.total_lp_iters; + const i_t bnb_lp_iters = exploration_stats_.total_lp_iters; + const f_t factor = settings_.diving_settings.iteration_limit_factor; + const f_t max_iter_f = factor * static_cast<f_t>(bnb_lp_iters); + const f_t rem_f = max_iter_f - static_cast<f_t>(stats.total_lp_iters); + const i_t max_i = std::numeric_limits<i_t>::max(); + lp_settings.iteration_limit = std::isfinite(rem_f) + ? static_cast<i_t>(std::min(rem_f, static_cast<f_t>(max_i))) + : max_i;
791-809: Add runtime guard for invalid branch_var before indexing.
branch_varis asserted (Line 795), but in Release builds this can still be-1, leading to OOB access inleaf_solution.x[branch_var]. Add a runtime check and return a safe status when variable selection fails.
As per coding guidelines, validate branch-and-bound decision correctness.🔧 Suggested guard
- assert(branch_var >= 0); - assert(round_dir != rounding_direction_t::NONE); + if (branch_var < 0 || round_dir == rounding_direction_t::NONE) { + search_tree.graphviz_node(log, node_ptr, "numerical", 0.0); + search_tree.update(node_ptr, node_status_t::NUMERICAL); + return {node_status_t::NUMERICAL, rounding_direction_t::NONE}; + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cpp/src/dual_simplex/branch_and_bound.cpp
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cu,cuh,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh,cpp,hpp,h}: Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Check that hard-coded GPU de...
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cpp,hpp,h}: Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Validate input sanitization to prevent buffer overflows and resource exhaustion attacks; avoid unsafe deserialization of problem files
Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cu,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Test with free variables, singleton problems, and extreme problem dimensions near resource limits to validate edge case handling
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2026-01-15T20:41:51.616Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 697
File: cpp/src/dual_simplex/diving_heuristics.cpp:200-228
Timestamp: 2026-01-15T20:41:51.616Z
Learning: In cpp/src/dual_simplex/diving_heuristics.cpp, the calculate_variable_locks function incorrectly indexes lp_problem.lower[nz_row] and lp_problem.upper[nz_row] where nz_row is a row index (0 to num_rows-1), but lower and upper are variable bounds of size num_cols indexed by column. This causes semantic errors and potential out-of-bounds access when num_rows > num_cols. Since lp_problem_t constraints are always equalities (A*x = rhs) after presolve, lock counting should treat each nonzero as contributing equally to both up_lock and down_lock, rather than checking constraint bounds.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cpp,hpp,h} : Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify error propagation from CUDA to user-facing APIs is complete; ensure CUDA errors are caught and mapped to meaningful user error codes
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cpp,hpp,h} : Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.cu : Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Write tests validating numerical correctness of optimization results (not just 'runs without error'); test degenerate cases (infeasible, unbounded, empty, singleton problems)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-12-04T04:11:12.640Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/src/dual_simplex/scaling.cpp:68-76
Timestamp: 2025-12-04T04:11:12.640Z
Learning: In the cuOPT dual simplex solver, CSR/CSC matrices (including the quadratic objective matrix Q) are required to have valid dimensions and indices by construction. Runtime bounds checking in performance-critical paths like matrix scaling is avoided to prevent slowdowns. Validation is performed via debug-only check_matrix() calls wrapped in `#ifdef` CHECK_MATRIX.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-12-04T20:09:09.264Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 602
File: cpp/src/linear_programming/solve.cu:732-742
Timestamp: 2025-12-04T20:09:09.264Z
Learning: In cpp/src/linear_programming/solve.cu, the barrier solver does not currently return INFEASIBLE or UNBOUNDED status. It only returns OPTIMAL, TIME_LIMIT, NUMERICAL_ISSUES, or CONCURRENT_LIMIT.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
🧬 Code graph analysis (1)
cpp/src/dual_simplex/branch_and_bound.cpp (8)
cpp/src/dual_simplex/presolve.cpp (3)
convert_user_problem(559-814)convert_user_problem(559-563)convert_user_problem(1510-1515)cpp/src/dual_simplex/branch_and_bound.hpp (9)
obj(188-188)symbol(189-189)solution(98-98)solution(115-115)solution(192-192)node_ptr(240-252)node_ptr(257-263)node_ptr(266-269)node(206-206)cpp/src/dual_simplex/solve.cpp (6)
compute_user_objective(88-93)compute_user_objective(88-88)compute_user_objective(96-100)compute_user_objective(96-96)compute_user_objective(661-662)compute_user_objective(664-664)cpp/src/dual_simplex/solve.hpp (2)
compute_user_objective(37-37)compute_user_objective(40-40)cpp/src/dual_simplex/pseudo_costs.hpp (4)
node_ptr(31-31)fractional(46-49)fractional(51-53)fractional(55-56)cpp/src/dual_simplex/mip_node.hpp (8)
node_ptr(277-283)node_ptr(277-277)log(329-337)log(329-332)log(339-354)log(339-344)node_status(173-192)node_status(173-173)cpp/src/dual_simplex/diving_heuristics.hpp (5)
coefficient_diving(51-56)line_search_diving(24-27)pseudocost_diving(30-34)guided_diving(37-41)calculate_variable_locks(46-48)cpp/src/dual_simplex/node_queue.hpp (4)
node(28-32)node(28-28)node(34-38)node(34-34)
🔇 Additional comments (12)
cpp/src/dual_simplex/branch_and_bound.cpp (12)
8-24: Header reorder only.
197-221: Feasible-solution symbol mapping looks consistent.
225-238: Constructor initialization is clear and correct.
248-259: Lower-bound aggregation with node_queue integration is sound.
262-280: Heuristic reporting behavior looks fine.
452-517: Final-solution reporting flow looks good.
520-545: Feasible-solution update path is clean.
547-566: Martin criteria helper looks good.
1132-1219: Diving traversal flow is coherent.
1222-1279: Diving thread setup and bound handling look good.
1359-1594: Solve orchestration and strategy wiring look solid.
1081-1129: Remove this comment — variables are already properly synchronized.Both
is_runningandactive_subtrees_are declared asomp_atomic_t<bool>andomp_atomic_t<i_t>respectively (branch_and_bound.hpp:176, 180), which provides thread-safe atomic operations via OpenMP. No data race exists here.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cpp/src/dual_simplex/branch_and_bound.cpp (1)
283-301: Division by zero and potential format specifier mismatch.
Line 289:
exploration_stats_.total_lp_iters / nodes_exploredcan divide by zero ifreport()is called before any nodes are explored.Line 291:
%10luformat specifier assumesnodes_unexploredisunsigned long, buti_tis typicallyint. This causes undefined behavior on some platforms.🐛 Proposed fix
void branch_and_bound_t<i_t, f_t>::report(char symbol, f_t obj, f_t lower_bound, i_t node_depth) { i_t nodes_explored = exploration_stats_.nodes_explored; i_t nodes_unexplored = exploration_stats_.nodes_unexplored; f_t user_obj = compute_user_objective(original_lp_, obj); f_t user_lower = compute_user_objective(original_lp_, lower_bound); - f_t iter_node = exploration_stats_.total_lp_iters / nodes_explored; + f_t iter_node = nodes_explored > 0 + ? static_cast<f_t>(exploration_stats_.total_lp_iters) / nodes_explored + : 0.0; std::string user_gap = user_mip_gap<f_t>(user_obj, user_lower); - settings_.log.printf("%c %10d %10lu %+13.6e %+10.6e %6d %7.1e %s %9.2f\n", + settings_.log.printf("%c %10d %10d %+13.6e %+10.6e %6d %7.1e %s %9.2f\n", symbol, nodes_explored, nodes_unexplored,
🤖 Fix all issues with AI agents
In `@cpp/src/dual_simplex/branch_and_bound.cpp`:
- Around line 640-646: The multiplication factor * bnb_lp_iters can overflow
when converting from f_t to i_t; update the calculation in the branch that
checks thread_type != bnb_worker_type_t::BEST_FIRST to perform the
multiplication in floating point, clamp the result to
std::numeric_limits<i_t>::max() before converting to i_t (use
exploration_stats_.total_lp_iters and
settings_.diving_settings.iteration_limit_factor), then compute
lp_settings.iteration_limit = clamped_max_iter - stats.total_lp_iters while
ensuring the subtraction cannot underflow (if result <= 0 return
dual::status_t::ITERATION_LIMIT). Ensure you reference the symbols
exploration_stats_.total_lp_iters,
settings_.diving_settings.iteration_limit_factor, lp_settings.iteration_limit,
stats.total_lp_iters, and types i_t/f_t when making the safe conversion and
clamp.
♻️ Duplicate comments (3)
cpp/src/dual_simplex/branch_and_bound.cpp (3)
855-856: Data race onexploration_stats_fields accessed by concurrent OpenMP tasks.Multiple fields of
exploration_stats_(e.g.,nodes_unexplored,nodes_explored,nodes_since_last_log) are read and written by concurrent#pragma omp taskinvocations without synchronization. This causes data races and undefined behavior.For example:
- Line 856:
--exploration_stats_.nodes_unexplored- Lines 911-913:
++nodes_since_last_log,++nodes_explored,--nodes_unexplored- Line 924:
nodes_unexplored += 2Consider using
std::atomicfor these counters or protecting access with a mutex (similar tomutex_upper_forupper_bound_).🐛 Suggested approach
Either:
- Make the counter fields in
exploration_stats_atomic:std::atomic<i_t> nodes_explored, etc.- Or add a dedicated mutex (e.g.,
mutex_stats_) and wrap all accesses inexploration_ramp_up,plunge_from, anddive_from.Option 1 is simpler for counters that only need atomic increment/decrement operations.
Also applies to: 911-914, 923-925
581-612: GUIDED_DIVING fallback ignores diving settings; enum cast missing for log.
Lines 581-584: The fallback unconditionally switches to
PSEUDOCOST_DIVINGwithout checking if it's enabled viasettings_.diving_settings.pseudocost_diving. If pseudocost diving is disabled, this should fall back to another enabled strategy.Line 609:
bnb_worker_type_tis anenum class, so%drequires an explicit cast to avoid undefined behavior.🐛 Proposed fix
// If there is no incumbent, use pseudocost diving instead of guided diving if (upper_bound_ == inf && type == bnb_worker_type_t::GUIDED_DIVING) { - type = bnb_worker_type_t::PSEUDOCOST_DIVING; + if (settings_.diving_settings.pseudocost_diving != 0) { + type = bnb_worker_type_t::PSEUDOCOST_DIVING; + } else if (settings_.diving_settings.line_search_diving != 0) { + type = bnb_worker_type_t::LINE_SEARCH_DIVING; + } else if (settings_.diving_settings.coefficient_diving != 0) { + type = bnb_worker_type_t::COEFFICIENT_DIVING; + } else { + return {-1, rounding_direction_t::NONE}; + } } // ... in default case: default: - log.debug("Unknown variable selection method: %d\n", type); + log.debug("Unknown variable selection method: %d\n", static_cast<int>(type)); return {-1, rounding_direction_t::NONE};
791-796: Add runtime check for branch_var to prevent undefined behavior in release builds.The
assert(branch_var >= 0)at line 795 is disabled in release builds. Ifvariable_selectionreturns-1(e.g., due to the GUIDED_DIVING fallback issue when all strategies are disabled), line 809 will accessleaf_solution.x[-1], causing undefined behavior.🐛 Proposed fix
auto [branch_var, round_dir] = variable_selection(node_ptr, leaf_fractional, leaf_solution.x, thread_type); assert(leaf_vstatus.size() == leaf_problem.num_cols); - assert(branch_var >= 0); - assert(round_dir != rounding_direction_t::NONE); + if (branch_var < 0 || round_dir == rounding_direction_t::NONE) { + log.printf("Variable selection failed for node %d\n", node_ptr->node_id); + search_tree.update(node_ptr, node_status_t::NUMERICAL); + return {node_status_t::NUMERICAL, rounding_direction_t::NONE}; + }
🧹 Nitpick comments (1)
cpp/src/dual_simplex/branch_and_bound.cpp (1)
1543-1546: Potential inconsistency between logged and actual diving worker counts.Line 1546 logs diving workers as
num_threads - num_bfs_workers, but line 1585 spawnssettings_.diving_settings.num_diving_workerstasks. If these values differ, the log message will be misleading.♻️ Suggested fix
settings_.log.printf("Exploring the B&B tree using %d threads (best-first = %d, diving = %d)\n", settings_.num_threads, settings_.num_bfs_workers, - settings_.num_threads - settings_.num_bfs_workers); + settings_.diving_settings.num_diving_workers);Also applies to: 1585-1586
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cpp/src/dual_simplex/branch_and_bound.cpp
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cu,cuh,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cu,cuh,cpp,hpp,h}: Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Check that hard-coded GPU de...
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
**/*.{cpp,hpp,h}: Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Validate input sanitization to prevent buffer overflows and resource exhaustion attacks; avoid unsafe deserialization of problem files
Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
**/*.{cu,cpp,hpp,h}
📄 CodeRabbit inference engine (.github/.coderabbit_review_guide.md)
Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Files:
cpp/src/dual_simplex/branch_and_bound.cpp
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate algorithm correctness in optimization logic: simplex pivots, branch-and-bound decisions, routing heuristics, and constraint/objective handling must produce correct results
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Validate correct initialization of variable bounds, constraint coefficients, and algorithm state before solving; ensure reset when transitioning between algorithm phases (presolve, simplex, diving, crossover)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Reduce tight coupling between solver components (presolve, simplex, basis, barrier); increase modularity and reusability of optimization algorithms
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Add tests for algorithm phase transitions: verify correct initialization of bounds and state when transitioning from presolve to simplex to diving to crossover
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2026-01-15T20:41:51.616Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 697
File: cpp/src/dual_simplex/diving_heuristics.cpp:200-228
Timestamp: 2026-01-15T20:41:51.616Z
Learning: In cpp/src/dual_simplex/diving_heuristics.cpp, the calculate_variable_locks function incorrectly indexes lp_problem.lower[nz_row] and lp_problem.upper[nz_row] where nz_row is a row index (0 to num_rows-1), but lower and upper are variable bounds of size num_cols indexed by column. This causes semantic errors and potential out-of-bounds access when num_rows > num_cols. Since lp_problem_t constraints are always equalities (A*x = rhs) after presolve, lock counting should treat each nonzero as contributing equally to both up_lock and down_lock, rather than checking constraint bounds.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Assess algorithmic complexity for large-scale problems (millions of variables/constraints); ensure O(n log n) or better complexity, not O(n²) or worse
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Identify assertions with overly strict numerical tolerances that fail on legitimate degenerate/edge cases (near-zero pivots, singular matrices, empty problems)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Test with free variables, singleton problems, and extreme problem dimensions near resource limits to validate edge case handling
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cpp,hpp,h} : Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify error propagation from CUDA to user-facing APIs is complete; ensure CUDA errors are caught and mapped to meaningful user error codes
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check numerical stability: prevent overflow/underflow, precision loss, division by zero/near-zero, and use epsilon comparisons for floating-point equality checks
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cpp,hpp,h} : Prevent thread-unsafe use of global and static variables; use proper mutex/synchronization in server code accessing shared solver state
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.cu : Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cpp,hpp,h} : Check for unclosed file handles when reading MPS/QPS problem files; ensure RAII patterns or proper cleanup in exception paths
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Write tests validating numerical correctness of optimization results (not just 'runs without error'); test degenerate cases (infeasible, unbounded, empty, singleton problems)
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-12-04T04:11:12.640Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/src/dual_simplex/scaling.cpp:68-76
Timestamp: 2025-12-04T04:11:12.640Z
Learning: In the cuOPT dual simplex solver, CSR/CSC matrices (including the quadratic objective matrix Q) are required to have valid dimensions and indices by construction. Runtime bounds checking in performance-critical paths like matrix scaling is avoided to prevent slowdowns. Validation is performed via debug-only check_matrix() calls wrapped in `#ifdef` CHECK_MATRIX.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
📚 Learning: 2025-12-04T20:09:09.264Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 602
File: cpp/src/linear_programming/solve.cu:732-742
Timestamp: 2025-12-04T20:09:09.264Z
Learning: In cpp/src/linear_programming/solve.cu, the barrier solver does not currently return INFEASIBLE or UNBOUNDED status. It only returns OPTIMAL, TIME_LIMIT, NUMERICAL_ISSUES, or CONCURRENT_LIMIT.
Applied to files:
cpp/src/dual_simplex/branch_and_bound.cpp
🧬 Code graph analysis (1)
cpp/src/dual_simplex/branch_and_bound.cpp (11)
cpp/src/dual_simplex/presolve.hpp (2)
convert_user_problem(112-116)uncrush_primal_solution(162-165)cpp/src/mip/diversity/recombiners/recombiner_stats.hpp (3)
i(94-100)stats(110-110)stats(112-112)cpp/src/dual_simplex/branch_and_bound.hpp (8)
obj(188-188)solution(98-98)solution(115-115)solution(192-192)node_ptr(240-252)node_ptr(257-263)node_ptr(266-269)node(206-206)cpp/src/dual_simplex/solve.cpp (10)
compute_user_objective(88-93)compute_user_objective(88-88)compute_user_objective(96-100)compute_user_objective(96-96)compute_user_objective(661-662)compute_user_objective(664-664)basic_list(113-113)compute_objective(76-85)compute_objective(76-76)compute_objective(658-659)cpp/src/dual_simplex/solve.hpp (3)
compute_user_objective(37-37)compute_user_objective(40-40)compute_objective(34-34)cpp/src/dual_simplex/pseudo_costs.hpp (4)
node_ptr(31-31)fractional(46-49)fractional(51-53)fractional(55-56)cpp/src/dual_simplex/mip_node.hpp (8)
node_ptr(277-283)node_ptr(277-277)log(329-337)log(329-332)log(339-354)log(339-344)node_status(173-192)node_status(173-173)cpp/src/dual_simplex/diving_heuristics.hpp (4)
coefficient_diving(51-56)line_search_diving(24-27)pseudocost_diving(30-34)guided_diving(37-41)cpp/src/dual_simplex/crossover.cpp (1)
basic_list(1120-1120)cpp/src/dual_simplex/phase2.cpp (1)
basic_list(2180-2180)cpp/src/dual_simplex/node_queue.hpp (4)
node(28-32)node(28-28)node(34-38)node(34-34)
🔇 Additional comments (8)
cpp/src/dual_simplex/branch_and_bound.cpp (8)
197-221: LGTM!The
feasible_solution_symbolfunction properly handles allbnb_worker_type_tenum cases with a sensible default fallback. The conditional compilation forSHOW_DIVING_TYPEprovides useful debugging capability while keeping production logs clean.
247-259: LGTM!The
get_lower_boundfunction correctly aggregates bounds from multiple sources (ceiling, node queue, local thread bounds) using atomic loads for thread safety.
1057-1063: LGTM!The stack ordering logic based on
round_dircorrectly prioritizes the preferred branching direction (UP or DOWN) by pushing it last so it's processed first (LIFO).
1136-1223: LGTM!The
dive_fromfunction correctly uses local state (dive_tree,dive_stats,stack) avoiding thread safety issues. The backtrack limit logic at line 1219 properly constrains exploration depth.
1256-1265: LGTM!The explicit locking around
node_queue_operations and thedetach_copy()pattern correctly handle concurrent access to the shared node queue.
1374-1396: LGTM!The diving strategy initialization correctly builds the vector based on settings and properly calculates variable locks only when coefficient diving is enabled. The warning message now includes the trailing newline.
1570-1590: LGTM!The
#pragma omp taskgroupcorrectly ensures ramp-up completes before starting worker threads. The empty check at line 1584 guards against division by zero in the modulo operation at line 1586.
1594-1599: LGTM!The finalization correctly computes the lower bound from the node queue (if not empty) or falls back to the root's lower bound, then sets the final solution.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
/merge |
The goal of this PR is to improve the diving framework in the MIP solver based on [1, Section 9.2]. It introduces the following changes:
node_queue) to allow the sharing of information between them. This also greatly reduces memory consumption (33GBvs48GBforneos-848589after250s) since the lower and upper variable no longer needs to be stored for diving (Unified Node Queue + Diving Node and Iteration Limits #718).MIPLIB results (GH200):
main (#718): 226 feasible solutions with 12.8% average primal gap
Reference:
[1] T. Achterberg, “Constraint Integer Programming,” PhD, Technischen Universität Berlin,
Berlin, 2007. doi: 10.14279/depositonce-1634.
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.